Skip to content

feat(django): Use functools.wraps in more places #4144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Mar 19, 2025
Merged

feat(django): Use functools.wraps in more places #4144

merged 7 commits into from
Mar 19, 2025

Conversation

@sentrivana
Copy link
Contributor

@sentrivana sentrivana commented Mar 18, 2025 •
edited
Loading

We're not using @functools.wraps in a handful of places in the Django integration, leading to the wrapped functions reporting wrong (i.e., Sentry wrapper) names when inspected.

Closes #4138

@sentrivana sentrivana changed the title feat(django): Use @wraps in more places feat(django): Use functools.wraps in more places Mar 18, 2025
@codecov
Copy link

codecov bot commented Mar 18, 2025 •
edited
Loading

❌ 782 Tests Failed:

Tests completed Failed Passed Skipped
21312 782 20530 4568
View the top 3 failed test(s) by shortest run time 
 tests.integrations.celery.integration_tests.test_celery_beat_cron_monitoring
Stack Traces | 0s run time 
.../hostedtoolcache/Python/3.7.17.../x64/lib/python3.7/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1006: in _gcd_import
    ???
<frozen importlib._bootstrap>:983: in _find_and_load
    ???
<frozen importlib._bootstrap>:953: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:219: in _call_with_frames_removed
    ???
<frozen importlib._bootstrap>:1006: in _gcd_import
    ???
<frozen importlib._bootstrap>:983: in _find_and_load
    ???
<frozen importlib._bootstrap>:967: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:677: in _load_unlocked
    ???
<frozen importlib._bootstrap_external>:728: in exec_module
    ???
<frozen importlib._bootstrap>:219: in _call_with_frames_removed
    ???
.../celery/integration_tests/__init__.py:7: in <module>
    from celery.beat import Scheduler
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/celery/beat.py:22: in <module>
    from . import __version__, platforms, signals
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/celery/signals.py:14: in <module>
    from .utils.dispatch import Signal
.tox/py3.7-celery-v5.0.5/lib/python3.7.../celery/utils/__init__.py:16: in <module>
    from .nodenames import nodename, nodesplit, worker_direct
.tox/py3.7-celery-v5.0.5/lib/python3.7.../celery/utils/nodenames.py:6: in <module>
    from kombu.entity import Exchange, Queue
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/kombu/entity.py:7: in <module>
    from .serialization import prepare_accept_content
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/kombu/serialization.py:440: in <module>
    for ep, args in entrypoints('kombu.serializers'):  # pragma: no cover
.tox/py3.7-celery-v5.0.5/lib/python3.7.../kombu/utils/compat.py:82: in entrypoints
    for ep in importlib_metadata.entry_points().get(namespace, [])
E   AttributeError: 'EntryPoints' object has no attribute 'get'
 tests.integrations.celery.test_celery
Stack Traces | 0s run time 
ImportError while importing test module '.../integrations/celery/test_celery.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/_pytest/python.py:617: in _importtestmodule
    mod = import_path(self.path, mode=importmode, root=self.config.rootpath)
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/_pytest/pathlib.py:567: in import_path
    importlib.import_module(module_name)
.../hostedtoolcache/Python/3.7.17.../x64/lib/python3.7/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1006: in _gcd_import
    ???
<frozen importlib._bootstrap>:983: in _find_and_load
    ???
<frozen importlib._bootstrap>:967: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:677: in _load_unlocked
    ???
.tox/py3.7-celery-v5.0.5/lib/python3.7.../_pytest/assertion/rewrite.py:186: in exec_module
    exec(co, module.__dict__)
.../integrations/celery/test_celery.py:6: in <module>
    from celery import Celery, VERSION
E   ImportError: cannot import name 'Celery' from 'celery' (.../sentry-python/sentry-python/.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/celery/__init__.py)
 tests.integrations.celery.test_update_celery_task_headers
Stack Traces | 0s run time 
.../integrations/celery/beat.py:28: in <module>
    from celery import Task, Celery  # type: ignore
E   ImportError: cannot import name 'Task' from 'celery' (.../sentry-python/sentry-python/.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/celery/__init__.py)

During handling of the above exception, another exception occurred:
.../integrations/celery/test_update_celery_task_headers.py:7: in <module>
    from sentry_sdk.integrations.celery import _update_celery_task_headers
.../integrations/celery/__init__.py:9: in <module>
    from sentry_sdk.integrations.celery.beat import (
.../integrations/celery/beat.py:37: in <module>
    raise DidNotEnable("Celery not installed")
E   sentry_sdk.integrations.DidNotEnable: Celery not installed

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@sentrivana sentrivana marked this pull request as ready for review March 18, 2025 10:22
szokeasaurusrex
szokeasaurusrex approved these changes Mar 18, 2025
View reviewed changes
Copy link
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, please address inline comments before merging

@sentrivana sentrivana enabled auto-merge (squash) March 18, 2025 12:30
@sentrivana sentrivana disabled auto-merge March 18, 2025 13:41
@sentrivana
Copy link
Contributor Author

sentrivana commented Mar 18, 2025

Looks like this change makes some unrelated tests fail/flake out. Investigating...

antonpirker
antonpirker approved these changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

One thought: could this confuse people because they are used to see some sentry_.. name and now suddenly the name changes?

@sentrivana
Copy link
Contributor Author

sentrivana commented Mar 18, 2025

One thought: could this confuse people because they are used to see some sentry_.. name and now suddenly the name changes?

It could. To be extra safe we could release this in a major 🤔

@antonpirker
Copy link
Contributor

antonpirker commented Mar 19, 2025

I guess this will also change the stack trace, so it could change the grouping. So maybe really better in a major version bump.

@sentrivana sentrivana changed the base branch from master to potel-base March 19, 2025 12:41
@sentrivana sentrivana force-pushed the ivana/functools-wraps-more-things branch from 2205120 to c6c1bf2 Compare March 19, 2025 12:45
@sentrivana
Copy link
Contributor Author

sentrivana commented Mar 19, 2025

Changed base to potel-base

@sentrivana sentrivana merged commit 88e6716 into potel-base Mar 19, 2025
89 of 125 checks passed
@sentrivana sentrivana deleted the ivana/functools-wraps-more-things branch March 19, 2025 12:52
@BYK BYK mentioned this pull request Sep 3, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@szokeasaurusrex szokeasaurusrex szokeasaurusrex approved these changes

+1 more reviewer

@antonpirker antonpirker antonpirker approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Django integration hides request handler function name

4 participants

@sentrivana @antonpirker @szokeasaurusrex